Correctness fixes - #242
Open
tomciopp wants to merge 3 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three correctness fixes, found by auditing the rounding paths after the
recent performance work. Each is its own commit with a pinning test. All
are wrong-answer bugs at the default context, reachable through the
public API with in-range decimal128 operands.
to_float/1: off by one for odd integers in [2^52, 2^53)
decimal_to_float/4rounded by comparing the remainder againstden >>> 1. Whenscale_downcollapses the denominator to 1, everyinput with a 53-bit significand, the division is exact, but the zero
remainder fails both
rem > denandrem < denagainst a halveddenominator of 0 and falls through to the ties-to-even clause, which
increments every odd quotient:
Fixed by comparing the doubled remainder against the denominator, which
is the rounding step as given in the algorithm the implementation
follows ("Correct Decimal to Floating-Point Using Big Integers",
exploringbinary.com, cited in
to_float/1), restoring IEEE 754correctly-rounded conversion. Odd values just above 2^53, where a zero
scaled remainder is a genuine tie, still round to even.
sqrt/1: discarded digits were not carried into rounding
The inexact branch of
do_sqrt/5passed its truncated root to thecontext with the sticky bit clear, so rounding treated the guard digit
as the entire discarded part, the same defect class as the division
sticky-bit fix (#236). The true root lies strictly beyond the truncated
coefficient on that branch, so:
precision 9 under
:ceiling,sqrt(10)returned 3.16227766, though√10 = 3.16227766016… ceils to 3.16227767
tie: at precision 2 under
:half_even,sqrt(1.57)returned 1.2instead of 1.3 (√1.57 = 1.25299…)
:rounded; they now also signal:inexactThe default
:half_upwas unaffected (the guard digit alone decidesthere), which is how this went unnoticed.
rem/2 and div_rem/2: remainder computed against a rounded product
Both derived the remainder as
sub(num1, mult(num2, quotient)), andmult/2applies the context, so when divisor × quotient exceeded thecontext precision, the remainder was computed against a rounded
multiple of the divisor. With 34-digit operands the rounded product can
equal the dividend exactly, cancelling the remainder:
The remainder is now computed exactly on raw coefficients (multiply,
align, subtract), with only the final result passing through the
context, per the General Decimal Arithmetic spec's requirement that
remainder is exact. Intermediates stay input-proportional because
integer_division/5caps the quotient at precision + 1 digits.Behavior note: a zero remainder now takes the dividend's sign
(
rem(-4, 2)is-0, previously+0), matching IEEE 754 and Python'sdecimal — it falls out of the exact computation, since the quotient is
floor(|num1| / |num2|).🤖 Generated with Claude Code